Skip to content

fix filter for next round period 0 votes#62

Merged
Vervious merged 5 commits intoalgorand:masterfrom
Vervious:nextvotefiltering
Jun 20, 2019
Merged

fix filter for next round period 0 votes#62
Vervious merged 5 commits intoalgorand:masterfrom
Vervious:nextvotefiltering

Conversation

@Vervious
Copy link
Copy Markdown
Contributor

Summary

This fixes a deviation from the spec. Currently, the code (accidentally) filters all votePresent and voteVerified events from the next round, if myPlayer.Period is large. Instead, whether we filter votes from the next round should not be a function of current period. As specified in the spec, we allow votes with vote.Round = player.Round + 1 and vote.Period == 0 and vote.Step = {propose,soft,cert,next}. We continue to filter all FPR votes.

Test Plan

Added a unit test for this scenario

Vervious added 3 commits June 13, 2019 15:04
This fixes a deviation from the spec. Currently, the code
(accidentally) filters all votePresent and voteVerified events
from the next round, if myPlayer.Period is large. Instead,
whether we filter votes from the next round should not be a
function of current period. As specified in the spec, we allow
votes with vote.Round = player.Round + 1 and vote.Period == 0
and vote.Step = {propose,soft,cert,next}
@Vervious Vervious self-assigned this Jun 19, 2019
@Vervious Vervious requested review from algoradam and derbear June 19, 2019 19:29
@Vervious Vervious added bug Something isn't working protocol labels Jun 19, 2019
@Vervious
Copy link
Copy Markdown
Contributor Author

Fixes #63

Comment thread agreement/voteAggregator_test.go
Comment thread agreement/voteAggregator.go Outdated
Copy link
Copy Markdown
Contributor

@derbear derbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(See comments)

@Vervious
Copy link
Copy Markdown
Contributor Author

See algorandfoundation/specs#1 for spec update

@Vervious Vervious requested a review from derbear June 20, 2019 18:00
@Vervious Vervious merged commit c05b79d into algorand:master Jun 20, 2019
@Vervious Vervious deleted the nextvotefiltering branch June 20, 2019 19:25
pzbitskiy pushed a commit to pzbitskiy/go-algorand that referenced this pull request May 1, 2020
Add goal app create --on-completion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants